Skip to content

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Jan 14, 2025

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-46394
Staging - https://deploy-preview-83--docs-mongoid.netlify.app/interact-data/crud/#dirty-tracking

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for docs-mongoid ready!

Name Link
🔨 Latest commit b527646
🔍 Latest deploy log https://app.netlify.com/sites/docs-mongoid/deploys/6787cf3156570c00080985d9
😎 Deploy Preview https://deploy-preview-83--docs-mongoid.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rustagir rustagir marked this pull request as ready for review January 14, 2025 19:28
Copy link
Contributor

@jordan-smith721 jordan-smith721 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Just a couple things but LGTM after those.

View Previous Changes
~~~~~~~~~~~~~~~~~~~~~

After you persist a model to MongoDB, you can still see what changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: clarify that mongoid clears changes after saving to highlight the usefulness of this

Suggested change
After you persist a model to MongoDB, you can still see what changes
After you persist a model to MongoDB, {+odm+} clears the current changes. However, you can still see what changes

Comment on lines +745 to +751
Update Container Fields
-----------------------

{+odm+} currently has an issue that prevents changes to attributes of
container types, such as ``Set`` or ``Array``, from saving to MongoDB.
You must assign all fields, including container types, for their values
to save to MongoDB.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add some kind of "Limitations" page for things like this? It seems weird to have an issue like this nested so far in a page. No action needed on this ticket but just wanted to raise the question.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! That could be a backlog ticket that we could file outside of the standardization. i can make it now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

person.reload.readonly? # => true

- If this flag is turned ``on``, you can mark a document as read-only
when that document is projected, such as by using ``only`` or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Such as by using" was really tripping me up here. I think this conveys the same idea

Suggested change
when that document is projected, such as by using ``only`` or
when that document is projected, by using methods such as, ``only`` or

Comment on lines 800 to 802
``without``. The resulting read-only document is not deletable or
destroyable, as {+odm+} raises a ``ReadonlyDocument`` error, but it is
saveable and updatable. The read-only status *is reset* if you reload the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
``without``. The resulting read-only document is not deletable or
destroyable, as {+odm+} raises a ``ReadonlyDocument`` error, but it is
saveable and updatable. The read-only status *is reset* if you reload the
``without``. As a result, you can't delete or destroy the read-only document,
as {+odm+} raises a ``ReadonlyDocument`` error, but you can
save and update it. The read-only status *is reset* if you reload the

Comment on lines 812 to 816
.. tip:: Projection

To learn more about projections, see the
:ref:`mongoid-data-projection` section of the Modify Query
Results guide.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be indented to go under the second bullet?

Comment on lines 818 to 822
Override readonly?
~~~~~~~~~~~~~~~~~~

You can also make a document read-only by overriding the ``readonly?``
method, as shown in the following code:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the previous section says: "You can make documents readonly in the following ways..." I think this can be a part of that section instead of having a dedicated header

Suggested change
Override readonly?
~~~~~~~~~~~~~~~~~~
You can also make a document read-only by overriding the ``readonly?``
method, as shown in the following code:
You can also make a document read-only by overriding the ``readonly?``
method, as shown in the following code:

@rustagir rustagir merged commit 78cb913 into mongodb:standardized Jan 15, 2025
4 of 5 checks passed
rustagir added a commit that referenced this pull request Jan 28, 2025
* DOCSP-42732: qs

* fix

* wip

* remove action

* NR suggestion

* DOCSP-42732: qs download

* wip

* adapt for sinatra

* vale fix

* snooty landing page

* MW PR fixes 1

* DOCSP-42733: atlas prep qs

* MW PR fixes 1

* DOCSP-42735: configure cxn

* small fix - vale

* JS PR fixes 1

* DOCSP-44008: read/write sinatra quickstart

* fixes

* NR PR fixes 1

* DOCSP-43961: rails qs

* vale

* ordering

* small fix

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* DOCSP-44647: add to existing app

* fix vale action

* vale fixes

* depth

* MW PR fixes 1

* fixes

* DOCSP-42745: interact with data drawer

* tags

* fix vale action

* remove extra word

Co-authored-by: Nora Reidy <[email protected]>

* DOCSP-42753: specify query part 1

* vale

* title

* code edits

* MW PR fixes 2

* DOCSP-44849: modify results

* vale

* JS PR fixes 1

* fix

* fix

* list fixes

* MW PR fixes 1

* DOCSP-44954: scoping

* add landing page

* link

* vale

* highlighting

* DOCSP-44821: specify a query pt 2

* MR PR fixes 1

* wip

* wip

* wip

* small fixes

* DOCSP-42767 Aggregation (#57)

* DOCSP-42774: transactions

* vale

* link text

* MW PR fixes 1

* MR PR fixes 1

* DOCSP-45306: model data drawer

* DOCSP-45330: inheritance (WIP)

* MR PR fixes 2

* try using roles

* wip

* vale

* add label

* fixes

* fix

* small fixes - MW

* DOCSP-45358: documents

* fix

* wip

* wip

* DR tech review 1

* page fmt

* page fmt

* SA PR fixes 1

* MR PR fixes 1

* DR small fix

Co-authored-by: Dmitry Rybakov <[email protected]>

* DOCSP-45360: nested attributes (#71)

* DOCSP-45360: nested attributes

* vale + fixes

* fixes

* NR PR fixes 1

* DOCSP-45362: text search (#72)

* DOCSP-45362: text search

* wip

* vale

* MM PR fixes 1

* DOCSP-45436 Field Behaviors page (#68)

* DOCSP-45363: validation (#73)

* DOCSP-45363: validation

* keywords

* wip

* SA PR fixes 1

* DOCSP-44794 Field Types (#69)

* DOCSP-45357 Sharding Configuration (#76)

* DOCSP-42762: Indexes (#74)

* DOCSP-45367 Associations pt. 1 (#79)

* DOCSP-45368: Persistence Configuration (#77)

* DOCSP-45361: callbacks (#75)

* DOCSP-45361: callbacks

* wip

* wip

* wip

* NR PR fixes 1

* DOCSP-45364: CRUD pt 1 (#81)

* checkpoint

* checkpoint 2

* woohoo first pass

* indent

* Edits

* updates

* vale chekcs

* RR PR fixes 1

* fix code file

* code fixes

* RM PR fixes 1

---------

Co-authored-by: rustagir <[email protected]>

* DOCSP-45110: queries subsections (#80)

* DOCSP-45110: queries misc sections

* wip:

* vale

* MR PR fixes 1

* GM PR fixes 1

* DOCSP-46072 Associations part 2 (#82)

* DOCSP-46394: CRUD remaining sections (#83)

* DOCSP-46394: CRUD remaining sections

* vale fixes

* JS PR fixes 1

* DOCSP-46213: bump to rails 8 and remove old tuts (#84)

* DOCSP-45356: i&h + code doc (#86)

* DOCSP-45356: i&h + code doc

* remove contributing

* vale fixes

* link fix

* vale fixes + RM comment

* DOCSP-42770: release notes/whats new (#87)

* DOCSP-42770: release notes/whats new

* fixes

* fixes

* DOCSP-42773: api links (#88)

* DOCSP-42773: api links

* fix

* link fixes

* DOCSP-42772: compatibility (#90)

* DOCSP-42772: compatibility

* small fix

* small fix

* SA PR fixes 1

* delete files for old build system

* column width adjustment

* DOCSP-45366 Encryption (#89)

* DOCSP-42741: config pages (#91)

* DOCSP-42741: config

* wip

* wip

* some vale fixes

* RM PR fixes 1

* small fix

* DOCSP-46555: rails integration (#92)

* wip

* DOCSP-46555: rails integration

* RM PR fixes 1

* DOCSP-45359 External Resources (#94)

* add additional resources page

* edits

* feedback

* DOCSP-42743 Collection config (#95)

* DOCSP-42730: landing page (#96)

* DOCSP-42730: landing page

* MW PR fixes 1

* small fix

* small fix

* small fix

* DOCSP-46121: cleanup (#97)

* cleanup

* copy compat action

* redirects

* MW PR fixes 1

* add index section

* change vs in redirects

---------

Co-authored-by: Nora Reidy <[email protected]>
Co-authored-by: Jordan Smith <[email protected]>
Co-authored-by: Dmitry Rybakov <[email protected]>
Co-authored-by: Maya Raman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants